Skip to content

fix initializer: configPath should never be inside /src and should be .js #64

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 1, 2021

Conversation

kiprasmel
Copy link
Contributor

im working on another improvement and having the config in .ts not .js makes it harder to work with before we run ts-node and get the built output, and this will be the same for consumers so I believe this is the better way for everyone.

wdyt?
& did i miss any spots for codeshift.config[tj]s?

Signed-off-by: Kipras Melnikovas <kipras@kipras.org>
@@ -113,7 +113,7 @@ export function initDirectory(
const transformPath = `${basePath}${!isReduced ? '/src/' : ''}/${transform}`;
const configPath = `${basePath}${
!isReduced ? '/src' : ''
}/codeshift.config.ts`;
}/codeshift.config.js`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @kiprasmel 👋 You should be able to initialise and simply switch over to a js codeshift.config.js file.
I'd prefer to have TS be the default output type to be consistent and typesafe.

Are you able to share more about the issue you're running into? maybe I can help 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey hey, that's fair, but i still think the config being js is a better default.

i am a big typescript fan and have been using it almost for every project for the last few years, but this is different. this could very well be a json file, and we could call the require.resolve ourselves.
it has little to do w/ type safety, and a lot with making tools that could utilize it, easier and simpler, because importing json/js is way easier than uncompiled .ts:D

i have created a wrapper cli around codeshift-cli just for simple running of local files, and having the js file as config makes it easy to import w/o having to compile or run ts-node first. the whole cli is temporary since we only need it for quickly running the local codemods but hopefully after more usage i can upstream the changes here. sadly cannot share atm since it depends on other unrelated code..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could very well be a json file, and we could call the require.resolve ourselves.
it has little to do w/ type safety, and a lot with making tools that could utilize it, easier and simpler, because importing json/js is way easier than uncompiled .ts:D

I really like this rationale! I'm onboard!

the whole cli is temporary since we only need it for quickly running the local codemods but hopefully after more usage i can upstream the changes here

Sounds amazing, i'd love that!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi, this is an example of why i needed this: #65

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants